Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue 646 install #680

Merged
merged 21 commits into from
Feb 12, 2020
Merged

Issue 646 install #680

merged 21 commits into from
Feb 12, 2020

Conversation

valentinsulzer
Copy link
Member

Description

Update Linux instructions with @jedgedrudd 's comments and add instructions for MacOS

Fixes #646
Fixes #673

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s.

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist:

  • No style issues: $ flake8
  • All tests pass: $ python run-tests.py --unit
  • The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@valentinsulzer valentinsulzer changed the base branch from master to issue-674-options-notebook October 25, 2019 01:45
@valentinsulzer
Copy link
Member Author

Accidentally created this branch from #674 so just going to merge into there

@codecov
Copy link

codecov bot commented Oct 25, 2019 •
edited
Loading

Codecov Report

Merging #680 into master will increase coverage by 0.04%.
The diff coverage is 98.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   98.14%   98.19%   +0.04%     
==========================================
  Files         180      181       +1     
  Lines        9987    10243     +256     
==========================================
+ Hits         9802    10058     +256     
  Misses        185      185
Impacted Files Coverage Δ
pybamm/expression_tree/exceptions.py 100% <ø> (ø) ⬆️
pybamm/models/submodels/particle/base_particle.py 93.33% <ø> (-3.97%) ⬇️
...dels/submodels/particle/fast/base_fast_particle.py 100% <ø> (ø) ⬆️
pybamm/solvers/casadi_solver.py 100% <100%> (ø) ⬆️
...m/models/full_battery_models/base_battery_model.py 100% <100%> (ø) ⬆️
...bamm/parameters/standard_parameters_lithium_ion.py 100% <100%> (ø) ⬆️
...ernal_circuit/function_control_external_circuit.py 100% <100%> (ø) ⬆️
...els/submodels/particle/fast/fast_many_particles.py 100% <100%> (ø) ⬆️
...odels/full_battery_models/lithium_ion/basic_dfn.py 100% <100%> (ø) ⬆️
...odels/full_battery_models/lithium_ion/basic_spm.py 100% <100%> (ø) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45adb23...c477420. Read the comment docs.

@valentinsulzer valentinsulzer changed the base branch from issue-674-options-notebook to master October 25, 2019 17:24
@valentinsulzer
Copy link
Member Author

@tlestang is it still worth working on this or will you sort out Mac and Windows install using pip?

martinjrobins
martinjrobins reviewed Jan 13, 2020
View reviewed changes
Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These instructions look good to me. Just need to sort out the errors on the travis mac build...

Scottmar93
Scottmar93 approved these changes Feb 12, 2020
View reviewed changes
Copy link
Contributor

@Scottmar93 Scottmar93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, will trust you that this works on macs :)

@valentinsulzer valentinsulzer merged commit b3d90d7 into master Feb 12, 2020
@valentinsulzer valentinsulzer deleted the issue-646-install branch February 12, 2020 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scottmar93 Scottmar93 Scottmar93 approved these changes

@jedgedrudd jedgedrudd Awaiting requested review from jedgedrudd

@martinjrobins martinjrobins Awaiting requested review from martinjrobins

@rtimms rtimms Awaiting requested review from rtimms

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update Linux installation notes Bug on Mac install (ft. sinister error message)
3 participants
@valentinsulzer @martinjrobins @Scottmar93